Add Storybook test runner with a11y (axe) and CI workflow#36
Conversation
- Add test-storybook and test-storybook:ci scripts - Add @storybook/test-runner and axe-playwright - Add .storybook/test-runner.ts for a11y checks (with a11y.disable opt-out) - Add GitHub Actions workflow to run UI tests on push (Playwright image) Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
bf5889b to
9998f68
Compare
📝 WalkthroughWalkthroughThis PR establishes a complete Storybook testing and build infrastructure while normalizing formatting across the codebase. It adds accessibility testing via axe-playwright with a CI workflow, updates Storybook and webpack configuration for TypeScript support and CSS extraction, and reformats all story, component, provider, and configuration files with consistent line endings and whitespace. ChangesStorybook Testing and Build Infrastructure
Storybook Story Files Formatting
UI Component Implementation Formatting
Theme Provider and Export System Formatting
Documentation and Workflows
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/components/ui/slider.tsx (1)
27-41: 💤 Low valueConsider removing commented-out code.
The commented
trackClassfunction spans 15 lines. If this code is no longer needed, removing it would improve readability. If it might be needed in the future, consider documenting why it's preserved in a TODO comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/slider.tsx` around lines 27 - 41, Remove the large commented-out block for the trackClass helper in src/components/ui/slider.tsx (the commented function named trackClass); either delete it entirely to improve readability or, if you want to keep it, replace the block with a one-line TODO comment explaining why it’s preserved and where an equivalent implementation lives, referencing trackClass so reviewers can locate the intent quickly..github/workflows/test-ui.yml (1)
19-20: ⚡ Quick winUse
npm cifor reproducible CI installs.Line 20 should use lockfile-based install to avoid dependency drift between runs.
Proposed change
- - name: Install dependencies - run: npm install + - name: Install dependencies + run: npm ci🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test-ui.yml around lines 19 - 20, The CI workflow step named "Install dependencies" currently runs npm install which can produce non-reproducible installs; change that step to run npm ci to perform a lockfile-based, deterministic install (and optionally ensure package-lock.json or npm-shrinkwrap.json is committed). Update the step that executes the install command (the "Install dependencies" job/step) to use npm ci instead of npm install, and add a brief check or comment to fail early if no lockfile is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/STORYBOOK.md`:
- Line 271: The doc references the preview config with two different filenames
causing confusion; open STORYBOOK.md and standardize all mentions to the actual
preview filename used in the repo (replace `.storybook/preview.js` occurrences
with `.storybook/preview.tsx` if the project uses TypeScript, or vice versa if
the real file is `.storybook/preview.js`), ensuring the references at the
currently mismatched spots (the lines referencing `.storybook/preview.tsx` and
the later `.storybook/preview.js`) are updated so every mention uses the single
correct filename.
In `@package.json`:
- Line 51: The package.json script "test-storybook:ci" uses CLI tools that
aren't declared as devDependencies; add the missing devDependencies for
"concurrently", "wait-on", and "http-server" (as devDependencies) so CI installs
them on fresh installs and the "test-storybook:ci" script can run; update
package.json's devDependencies section accordingly and run npm/yarn install to
verify.
In `@src/components/ui/index.ts`:
- Around line 64-69: Update the inconsistent quote and semicolon style in the UI
barrel exports: change the single quotes to double quotes and add trailing
semicolons for the exports that use single quotes currently — specifically the
export lines that include Toggle, ToggleGroup, ToggleGroupItem, Tabs, TabsList,
TabsTrigger, TabsContent, tabsListVariants, SelectionType, SelectionItem, and
selectionItemVariants — so all export statements match the double-quote +
semicolon style used elsewhere in this file.
In `@src/providers/theme-provider.tsx`:
- Around line 470-473: Replace the template-string class concatenation for the
component's root element with the shared cn() utility: when setting className
use cn(...) to merge "pui-root", the conditional dark class (based on
resolvedMode === "dark"), and the incoming className prop; update the line that
currently constructs className with `` `pui-root ${resolvedMode === "dark" ?
"dark" : ""} ${className}`.trim()`` to call cn("pui-root", resolvedMode ===
"dark" && "dark", className) so class merging follows the project's
cn/clsx+tw-merge convention.
In `@src/themes/index.ts`:
- Around line 6-125: The theme tokens in this file (e.g., defaultTheme,
defaultDarkTheme, dokanTheme, dokanDarkTheme, blueTheme, blueDarkTheme,
greenTheme, greenDarkTheme, amberTheme, amberDarkTheme, slateTheme,
slateDarkTheme) use "oklch(...)" strings but must be converted to the HSL
raw-value contract (plain "H S% L%" channel strings without the hsl() wrapper).
Update every color token (primary, primaryForeground, ring, background,
foreground, card, cardForeground, secondary, secondaryForeground, muted,
mutedForeground, border, input, etc.) to their equivalent HSL raw channel
strings (leave radius values untouched), ensuring all dark theme spreads
(...defaultDarkTheme) also receive converted HSL values.
In `@webpack.config.js`:
- Around line 19-28: The externals block currently replaces
defaultConfig.externals, removing `@wordpress/scripts` defaults; change it to
merge with defaults by using the existing defaultConfig.externals and spreading
it (e.g., create externals = { ...defaultConfig.externals, react: 'react',
'react-dom': 'react-dom', '`@wordpress/element`': '`@wordpress/element`',
'`@wordpress/components`': '`@wordpress/components`', '`@wordpress/block-editor`':
'`@wordpress/block-editor`', '`@wordpress/hooks`': '`@wordpress/hooks`',
'`@wordpress/i18n`': '`@wordpress/i18n`', quill: 'quill' }) so you preserve defaults
while adding your custom entries; update the webpack config where externals is
set (referencing defaultConfig and the externals property) to apply this merged
object.
---
Nitpick comments:
In @.github/workflows/test-ui.yml:
- Around line 19-20: The CI workflow step named "Install dependencies" currently
runs npm install which can produce non-reproducible installs; change that step
to run npm ci to perform a lockfile-based, deterministic install (and optionally
ensure package-lock.json or npm-shrinkwrap.json is committed). Update the step
that executes the install command (the "Install dependencies" job/step) to use
npm ci instead of npm install, and add a brief check or comment to fail early if
no lockfile is present.
In `@src/components/ui/slider.tsx`:
- Around line 27-41: Remove the large commented-out block for the trackClass
helper in src/components/ui/slider.tsx (the commented function named
trackClass); either delete it entirely to improve readability or, if you want to
keep it, replace the block with a one-line TODO comment explaining why it’s
preserved and where an equivalent implementation lives, referencing trackClass
so reviewers can locate the intent quickly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 934228da-a3b7-4987-9176-35e967e76725
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (51)
.github/workflows/deploy-storybook.yml.github/workflows/test-ui.yml.gitignore.storybook/main.ts.storybook/preview.js.storybook/test-runner.tsREADME.mddocs/STORYBOOK.mdpackage.jsonpostcss.config.jssrc/components/ui/Alert.stories.tsxsrc/components/ui/Avatar.stories.tsxsrc/components/ui/Combobox.stories.tsxsrc/components/ui/DropdownMenu.stories.tsxsrc/components/ui/Input.stories.tsxsrc/components/ui/InputGroup.stories.tsxsrc/components/ui/InputOTP.stories.tsxsrc/components/ui/Progress.stories.tsxsrc/components/ui/SelectionType.stories.tsxsrc/components/ui/Slider.stories.tsxsrc/components/ui/Switch.stories.tsxsrc/components/ui/Textarea.stories.tsxsrc/components/ui/ToggleGroup.stories.tsxsrc/components/ui/Tooltip.stories.tsxsrc/components/ui/alert-dialog.tsxsrc/components/ui/alert.tsxsrc/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/ui/card.tsxsrc/components/ui/combobox.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/index.tssrc/components/ui/input-group.tsxsrc/components/ui/input.tsxsrc/components/ui/modal.tsxsrc/components/ui/notice.tsxsrc/components/ui/progress.tsxsrc/components/ui/select.tsxsrc/components/ui/selection-type.tsxsrc/components/ui/slider.tsxsrc/components/ui/switch.tsxsrc/components/ui/textarea.tsxsrc/components/ui/toggle-group.tsxsrc/components/ui/tooltip.tsxsrc/index.tssrc/lib/utils.tssrc/providers/theme-provider.tsxsrc/styles.csssrc/themes/index.tstsconfig.jsonwebpack.config.js
| ## 5. Decorators (global theme / layout) | ||
| All stories run inside the preview decorators (see `.storybook/preview.tsx`), which: |
There was a problem hiding this comment.
Fix inconsistent preview config filename references.
Line 271/383 point to .storybook/preview.tsx, but this doc later references .storybook/preview.js (Line 369). Please standardize these references to the actual file to avoid broken guidance.
Also applies to: 383-383
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/STORYBOOK.md` at line 271, The doc references the preview config with
two different filenames causing confusion; open STORYBOOK.md and standardize all
mentions to the actual preview filename used in the repo (replace
`.storybook/preview.js` occurrences with `.storybook/preview.tsx` if the project
uses TypeScript, or vice versa if the real file is `.storybook/preview.js`),
ensuring the references at the currently mismatched spots (the lines referencing
`.storybook/preview.tsx` and the later `.storybook/preview.js`) are updated so
every mention uses the single correct filename.
| "storybook": "storybook dev -p 6006", | ||
| "build-storybook": "storybook build", | ||
| "test-storybook": "test-storybook", | ||
| "test-storybook:ci": "concurrently -k -s first -n \"SB,TEST\" -c \"magenta,blue\" \"npm run build-storybook && npx http-server storybook-static --port 6006 --silent\" \"wait-on tcp:6006 && test-storybook\"", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether required CLI deps for test-storybook:ci are declared.
rg -n '"test-storybook:ci"' package.json
rg -n '"(concurrently|wait-on|http-server)"' package.jsonRepository: getdokan/plugin-ui
Length of output: 282
🏁 Script executed:
cat -n package.jsonRepository: getdokan/plugin-ui
Length of output: 4526
test-storybook:ci script depends on CLIs not declared in devDependencies.
Line 51's test-storybook:ci script invokes concurrently, wait-on, and http-server, but none of these packages are present in devDependencies. This will cause CI to fail on fresh installs.
Add the missing dependencies:
Proposed change
"devDependencies": {
+ "concurrently": "^9.0.1",
+ "wait-on": "^8.0.1",
+ "http-server": "^14.1.1",
"`@babel/preset-react`": "^7.26.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "test-storybook:ci": "concurrently -k -s first -n \"SB,TEST\" -c \"magenta,blue\" \"npm run build-storybook && npx http-server storybook-static --port 6006 --silent\" \"wait-on tcp:6006 && test-storybook\"", | |
| "devDependencies": { | |
| "concurrently": "^9.0.1", | |
| "wait-on": "^8.0.1", | |
| "http-server": "^14.1.1", | |
| "`@babel/preset-react`": "^7.26.0", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 51, The package.json script "test-storybook:ci" uses
CLI tools that aren't declared as devDependencies; add the missing
devDependencies for "concurrently", "wait-on", and "http-server" (as
devDependencies) so CI installs them on fresh installs and the
"test-storybook:ci" script can run; update package.json's devDependencies
section accordingly and run npm/yarn install to verify.
| export { Toggle } from './toggle' | ||
| export { ToggleGroup, ToggleGroupItem } from './toggle-group' | ||
| export { Progress, ProgressTrack, ProgressIndicator, ProgressLabel, ProgressValue, CircularProgress, type ProgressProps, type CircularProgressProps } from "./progress"; | ||
| export { Slider, type SliderProps } from "./slider"; | ||
| export { Tabs, TabsList, TabsTrigger, TabsContent, tabsListVariants } from './tabs' | ||
| export { SelectionType, SelectionItem, selectionItemVariants } from './selection-type' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Run ESLint on the index.ts file to check for quote-style violations
npm run lint -- src/components/ui/index.tsRepository: getdokan/plugin-ui
Length of output: 195
🏁 Script executed:
# Check for ESLint configuration and the actual file content
find . -maxdepth 2 -type f \( -name ".eslintrc*" -o -name "eslint.config.*" \) | head -20Repository: getdokan/plugin-ui
Length of output: 44
🏁 Script executed:
# Check the actual content of the file to verify quote styles
cat -n src/components/ui/index.ts | tail -20Repository: getdokan/plugin-ui
Length of output: 1157
🏁 Script executed:
# Check package.json to understand npm scripts and dependencies
cat package.json | grep -A 10 '"lint"'Repository: getdokan/plugin-ui
Length of output: 395
🏁 Script executed:
# Search more thoroughly for ESLint config
find . -type f -name ".eslintrc*" -o -name "eslint.config.*" 2>/dev/null | head -20Repository: getdokan/plugin-ui
Length of output: 44
🏁 Script executed:
# Check if there's an eslint config in package.json itself
cat package.json | grep -A 20 '"eslintConfig"'Repository: getdokan/plugin-ui
Length of output: 44
🏁 Script executed:
# Try to find all config files in root
ls -la | grep -E '^\.'Repository: getdokan/plugin-ui
Length of output: 44
🏁 Script executed:
# List all files in repository root
ls -la | head -30Repository: getdokan/plugin-ui
Length of output: 1078
🏁 Script executed:
# Search for any file containing eslint configuration
find . -maxdepth 3 -type f \( -name "*config*" -o -name "*.json" \) | grep -v node_modules | head -20Repository: getdokan/plugin-ui
Length of output: 204
🏁 Script executed:
# Check if eslint is in package.json dependencies
cat package.json | grep -i eslintRepository: getdokan/plugin-ui
Length of output: 108
🏁 Script executed:
# Check for .eslintignore and other hidden eslint files
ls -la | grep eslintRepository: getdokan/plugin-ui
Length of output: 44
🏁 Script executed:
# Search for any eslint references in the codebase
rg "eslint" --type json --type javascript --type typescript | head -20Repository: getdokan/plugin-ui
Length of output: 97
🏁 Script executed:
# Check .github directory for CI/CD config that might have eslint setup
find .github -type f -name "*.yml" -o -name "*.yaml" | xargs cat 2>/dev/nullRepository: getdokan/plugin-ui
Length of output: 2018
Fix quote style inconsistency.
Lines 64-65 and 68-69 use single quotes while the rest of the file uses double quotes. This inconsistency also appears with semicolons (missing on those same lines). Update to use consistent double quotes and semicolons throughout, matching the rest of the file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/index.ts` around lines 64 - 69, Update the inconsistent
quote and semicolon style in the UI barrel exports: change the single quotes to
double quotes and add trailing semicolons for the exports that use single quotes
currently — specifically the export lines that include Toggle, ToggleGroup,
ToggleGroupItem, Tabs, TabsList, TabsTrigger, TabsContent, tabsListVariants,
SelectionType, SelectionItem, and selectionItemVariants — so all export
statements match the double-quote + semicolon style used elsewhere in this file.
| className={`pui-root ${ | ||
| resolvedMode === "dark" ? "dark" : "" | ||
| } ${className}`.trim()} | ||
| style={{ ...cssVariables, ...style }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use cn() for class merging instead of template-string concatenation.
Line 470–Line 473 should use the shared merge utility for consistency and safer class composition.
Proposed change
import React, {
createContext,
useCallback,
useContext,
useEffect,
useMemo,
useState,
type ReactNode,
} from "react";
+import { cn } from "`@/lib/utils`";
@@
data-pui-plugin={pluginId}
data-pui-mode={resolvedMode}
- className={`pui-root ${
- resolvedMode === "dark" ? "dark" : ""
- } ${className}`.trim()}
+ className={cn("pui-root", resolvedMode === "dark" && "dark", className)}
style={{ ...cssVariables, ...style }}
>As per coding guidelines, "Use the cn() utility function to merge Tailwind CSS classes (combines clsx + tailwind-merge)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={`pui-root ${ | |
| resolvedMode === "dark" ? "dark" : "" | |
| } ${className}`.trim()} | |
| style={{ ...cssVariables, ...style }} | |
| className={cn("pui-root", resolvedMode === "dark" && "dark", className)} | |
| style={{ ...cssVariables, ...style }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/theme-provider.tsx` around lines 470 - 473, Replace the
template-string class concatenation for the component's root element with the
shared cn() utility: when setting className use cn(...) to merge "pui-root", the
conditional dark class (based on resolvedMode === "dark"), and the incoming
className prop; update the line that currently constructs className with ``
`pui-root ${resolvedMode === "dark" ? "dark" : ""} ${className}`.trim()`` to
call cn("pui-root", resolvedMode === "dark" && "dark", className) so class
merging follows the project's cn/clsx+tw-merge convention.
| export const defaultTheme: ThemeTokens = { | ||
| primary: "oklch(0.5410 0.2120 265.7540)", | ||
| primaryForeground: "oklch(1.0000 0 0)", | ||
| ring: "oklch(0.5410 0.2120 265.7540)", | ||
| radius: "0.5rem", | ||
| }; | ||
| /** | ||
| * Default dark theme tokens | ||
| */ | ||
| export const defaultDarkTheme: ThemeTokens = { | ||
| background: "oklch(0.1450 0.0140 285.7500)", | ||
| foreground: "oklch(0.9850 0.0030 264.5000)", | ||
| card: "oklch(0.1450 0.0140 285.7500)", | ||
| cardForeground: "oklch(0.9850 0.0030 264.5000)", | ||
| primary: "oklch(0.6230 0.2000 265.7540)", | ||
| primaryForeground: "oklch(0.1450 0.0140 285.7500)", | ||
| secondary: "oklch(0.2690 0.0150 264.5419)", | ||
| secondaryForeground: "oklch(0.9850 0.0030 264.5000)", | ||
| muted: "oklch(0.2690 0.0150 264.5419)", | ||
| mutedForeground: "oklch(0.7110 0.0200 264.3637)", | ||
| border: "oklch(0.3690 0.0150 264.5313)", | ||
| input: "oklch(0.3690 0.0150 264.5313)", | ||
| ring: "oklch(0.6230 0.2000 265.7540)", | ||
| }; | ||
| /** | ||
| * Dokan theme - Purple brand | ||
| */ | ||
| export const dokanTheme: ThemeTokens = { | ||
| primary: "oklch(0.5410 0.2120 265.7540)", // #7047EB | ||
| primaryForeground: "oklch(1.0000 0 0)", | ||
| ring: "oklch(0.5410 0.2120 265.7540)", | ||
| radius: "0.375rem", | ||
| }; | ||
| /** | ||
| * Dokan dark theme | ||
| */ | ||
| export const dokanDarkTheme: ThemeTokens = { | ||
| ...defaultDarkTheme, | ||
| primary: "oklch(0.6230 0.2000 265.7540)", | ||
| ring: "oklch(0.6230 0.2000 265.7540)", | ||
| }; | ||
| /** | ||
| * Blue theme - For plugins like WeMail | ||
| */ | ||
| export const blueTheme: ThemeTokens = { | ||
| primary: "oklch(0.6230 0.2140 255.0900)", // Blue | ||
| primaryForeground: "oklch(1.0000 0 0)", | ||
| ring: "oklch(0.6230 0.2140 255.0900)", | ||
| radius: "0.5rem", | ||
| }; | ||
| /** | ||
| * Blue dark theme | ||
| */ | ||
| export const blueDarkTheme: ThemeTokens = { | ||
| ...defaultDarkTheme, | ||
| primary: "oklch(0.7000 0.1800 255.0900)", | ||
| ring: "oklch(0.7000 0.1800 255.0900)", | ||
| }; | ||
| /** | ||
| * Green theme - For plugins with nature/growth branding | ||
| */ | ||
| export const greenTheme: ThemeTokens = { | ||
| primary: "oklch(0.6470 0.1780 145.0000)", // Green | ||
| primaryForeground: "oklch(1.0000 0 0)", | ||
| ring: "oklch(0.6470 0.1780 145.0000)", | ||
| radius: "0.5rem", | ||
| }; | ||
| /** | ||
| * Green dark theme | ||
| */ | ||
| export const greenDarkTheme: ThemeTokens = { | ||
| ...defaultDarkTheme, | ||
| primary: "oklch(0.7200 0.1600 145.0000)", | ||
| ring: "oklch(0.7200 0.1600 145.0000)", | ||
| }; | ||
| /** | ||
| * Orange/Amber theme - For warm branding | ||
| */ | ||
| export const amberTheme: ThemeTokens = { | ||
| primary: "oklch(0.7690 0.1880 70.0800)", // Amber/Orange | ||
| primaryForeground: "oklch(0.2100 0.0340 32.0000)", | ||
| ring: "oklch(0.7690 0.1880 70.0800)", | ||
| radius: "0.5rem", | ||
| }; | ||
| /** | ||
| * Amber dark theme | ||
| */ | ||
| export const amberDarkTheme: ThemeTokens = { | ||
| ...defaultDarkTheme, | ||
| primary: "oklch(0.8200 0.1600 70.0800)", | ||
| ring: "oklch(0.8200 0.1600 70.0800)", | ||
| }; | ||
| /** | ||
| * Slate/Neutral theme - For minimal, professional look | ||
| */ | ||
| export const slateTheme: ThemeTokens = { | ||
| primary: "oklch(0.3050 0.0170 264.5419)", // Slate | ||
| primaryForeground: "oklch(1.0000 0 0)", | ||
| ring: "oklch(0.3050 0.0170 264.5419)", | ||
| radius: "0.375rem", | ||
| }; | ||
| /** | ||
| * Slate dark theme | ||
| */ | ||
| export const slateDarkTheme: ThemeTokens = { | ||
| ...defaultDarkTheme, | ||
| primary: "oklch(0.7110 0.0200 264.3637)", | ||
| ring: "oklch(0.7110 0.0200 264.3637)", | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Theme token format is inconsistent with the required HSL raw-value contract.
Line 6 onward defines ThemeTokens with oklch(...) values; this should be migrated to raw HSL channel strings (without hsl() wrapper) to match the theme-system contract.
As per coding guidelines, "Theme system should use HSL values without the hsl() wrapper (e.g., '220 90% 56%') for all ThemeTokens".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/themes/index.ts` around lines 6 - 125, The theme tokens in this file
(e.g., defaultTheme, defaultDarkTheme, dokanTheme, dokanDarkTheme, blueTheme,
blueDarkTheme, greenTheme, greenDarkTheme, amberTheme, amberDarkTheme,
slateTheme, slateDarkTheme) use "oklch(...)" strings but must be converted to
the HSL raw-value contract (plain "H S% L%" channel strings without the hsl()
wrapper). Update every color token (primary, primaryForeground, ring,
background, foreground, card, cardForeground, secondary, secondaryForeground,
muted, mutedForeground, border, input, etc.) to their equivalent HSL raw channel
strings (leave radius values untouched), ensuring all dark theme spreads
(...defaultDarkTheme) also receive converted HSL values.
| externals: { | ||
| react: 'react', | ||
| 'react-dom': 'react-dom', | ||
| '@wordpress/element': '@wordpress/element', | ||
| '@wordpress/components': '@wordpress/components', | ||
| '@wordpress/block-editor': '@wordpress/block-editor', | ||
| '@wordpress/hooks': '@wordpress/hooks', | ||
| '@wordpress/i18n': '@wordpress/i18n', | ||
| quill: 'quill', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current config overrides externals instead of merging defaults.
rg -n "externals\\s*:" webpack.config.js -n -A12 -B2
rg -n "defaultConfig\\.externals" webpack.config.jsRepository: getdokan/plugin-ui
Length of output: 586
🏁 Script executed:
rg -n "defaultConfig" webpack.config.js -B5 -A5Repository: getdokan/plugin-ui
Length of output: 1714
🏁 Script executed:
rg -n "from.*`@wordpress/scripts`\|import.*defaultConfig" .Repository: getdokan/plugin-ui
Length of output: 44
🏁 Script executed:
fd webpack.config.js -x cat -n {} \; | head -40Repository: getdokan/plugin-ui
Length of output: 1640
🌐 Web query:
@wordpress/scripts webpack config externals defaults
💡 Result:
The @wordpress/scripts package provides a default Webpack configuration that automatically handles the externalization of WordPress core packages (e.g., @wordpress/components, @wordpress/element) [1][2]. This is primarily managed by the @wordpress/dependency-extraction-webpack-plugin, which is included in the default configuration [1]. Key points regarding externals and customization: 1. Default Behavior: The default configuration automatically maps WordPress package imports to their corresponding global variables (e.g., @wordpress/blob becomes wp.blob) [1]. This allows you to use import statements in your code without bundling the entire WordPress core library, ensuring your scripts rely on the versions already loaded by WordPress [1][3]. 2. Customizing Externals: If you need to modify or extend the default externals, you can create a webpack.config.js file in your project root [4][5]. You can import the default configuration and merge your changes [4][5]. 3. Conflicts with DependencyExtractionWebpackPlugin: The default plugin overlaps with standard Webpack externals [1]. If you add custom externals that conflict with the plugin's automatic extraction, you may need to remove the default instance of the DependencyExtractionWebpackPlugin from the plugins array and re-add it with custom options, or use the externals option directly if you do not need the automatic dependency list generation [1]. 4. Disabling Externals: If you are building a standalone application that does not run within the WordPress environment, you can invoke the build or start scripts with the --webpack-no-externals flag to prevent the automatic externalization of WordPress packages [6]. To extend the configuration, use the following pattern in your webpack.config.js: const defaultConfig = require( '@wordpress/scripts/config/webpack.config'); module.exports = {...defaultConfig, // Add your custom externals or overrides here externals: {...defaultConfig.externals, 'my-custom-library': 'MyGlobalVar', }, }; Always ensure you are pinning your @wordpress/scripts version, as major updates may restructure the internal configuration [4].
Citations:
- 1: https://developer.wordpress.org/block-editor/reference-guides/packages/packages-dependency-extraction-webpack-plugin
- 2: How Webpack and WordPress packages interact WordPress/developer-blog-content#15
- 3: https://webpack.js.org/configuration/externals/
- 4: https://jonimms.com/wordpress-build-tools-wp-scripts-beyond-defaults/
- 5: https://developer.wordpress.org/themes/advanced-topics/build-process
- 6: Can I use the default webpack config (wp-scripts) to mount elements in DOM? WordPress/gutenberg#27557
Do not replace defaultConfig.externals; merge it instead.
Line 19 replaces the entire externals map, which removes defaults from @wordpress/scripts. The correct pattern is to spread the defaults and add custom externals:
- externals: {
+ externals: {
+ ...defaultConfig.externals,
react: 'react',
'react-dom': 'react-dom',
'`@wordpress/element`': '`@wordpress/element`',This aligns with the pattern used for other config properties (output, module, plugins, resolve) and matches the official @wordpress/scripts documentation for extending configurations.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| externals: { | |
| react: 'react', | |
| 'react-dom': 'react-dom', | |
| '@wordpress/element': '@wordpress/element', | |
| '@wordpress/components': '@wordpress/components', | |
| '@wordpress/block-editor': '@wordpress/block-editor', | |
| '@wordpress/hooks': '@wordpress/hooks', | |
| '@wordpress/i18n': '@wordpress/i18n', | |
| quill: 'quill', | |
| }, | |
| externals: { | |
| ...defaultConfig.externals, | |
| react: 'react', | |
| 'react-dom': 'react-dom', | |
| '`@wordpress/element`': '`@wordpress/element`', | |
| '`@wordpress/components`': '`@wordpress/components`', | |
| '`@wordpress/block-editor`': '`@wordpress/block-editor`', | |
| '`@wordpress/hooks`': '`@wordpress/hooks`', | |
| '`@wordpress/i18n`': '`@wordpress/i18n`', | |
| quill: 'quill', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webpack.config.js` around lines 19 - 28, The externals block currently
replaces defaultConfig.externals, removing `@wordpress/scripts` defaults; change
it to merge with defaults by using the existing defaultConfig.externals and
spreading it (e.g., create externals = { ...defaultConfig.externals, react:
'react', 'react-dom': 'react-dom', '`@wordpress/element`': '`@wordpress/element`',
'`@wordpress/components`': '`@wordpress/components`', '`@wordpress/block-editor`':
'`@wordpress/block-editor`', '`@wordpress/hooks`': '`@wordpress/hooks`',
'`@wordpress/i18n`': '`@wordpress/i18n`', quill: 'quill' }) so you preserve defaults
while adding your custom entries; update the webpack config where externals is
set (referencing defaultConfig and the externals property) to apply this merged
object.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores